Skip to content

fix(core): surface registry configuration errors - #3077

Merged
ascorbic merged 2 commits into
mainfrom
codex/1350-registry-config-errors
Sep 12, 2026
Merged

fix(core): surface registry configuration errors#3077
ascorbic merged 2 commits into
mainfrom
codex/1350-registry-config-errors

Conversation

@ascorbic

Copy link
Copy Markdown
Collaborator

What does this PR do?

Invalid plugin registry settings now produce field-specific configuration errors while Astro loads the site. If a recognized invalid registry setting reaches runtime manifest generation, the admin remains available and shows which experimental.registry field to correct; unexpected manifest failures still return an error.

The diagnostic contains only a stable code and field name, so configured values and credentials are not exposed. The change adds no logged-out queries.

Part of #1350.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main.
  • I have added and reviewed the user-facing changeset (if this PR changes a published package)
  • New features link to an approved Discussion: n/a — bug fix
  • I have included screenshots below if this PR changes the UI

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: GPT-5.6

Screenshots / test output

Admin error banner directing the administrator to the invalid minimum release age field in astro.config.mjs

  • Core registry, integration, runtime, and API tests: 79 passed
  • Admin browser tests: 2 passed
  • Root build and typecheck passed
  • Type-aware lint: 0 diagnostics
  • Formatting, changeset validation, and diff checks passed

Admin error banner directing the administrator to the invalid minimum release age field in astro.config.mjs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 12, 2026

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://codex-1350-registry-config-errors.try.emdashcms.com, https://codex-1350-registry-config-errors-emdash-playground.emdash-cms.workers.dev (commit 72970d3)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://d6a8fa4e.try.emdashcms.com, https://d6a8fa4e-emdash-playground.emdash-cms.workers.dev 72970d3 2026-09-12T08:20:53.206Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://90a0e6ad.try.emdashcms.com, https://90a0e6ad-emdash-playground.emdash-cms.workers.dev 820e1da 2026-09-12T07:21:44.606Z Visit the dashboard ↗

@changeset-bot

changeset-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 72970d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
emdash Patch
@emdash-cms/admin Patch
@emdash-cms/cloudflare Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/do-demo-site Patch
@emdash-cms/do-solo-demo-site Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
create-emdash Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/release-service Patch
@emdash-cms/plugin-embeds Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@3077

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@3077

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@3077

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@3077

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@3077

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@3077

emdash

npm i https://pkg.pr.new/emdash@3077

create-emdash

npm i https://pkg.pr.new/create-emdash@3077

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@3077

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@3077

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@3077

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@3077

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@3077

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/@emdash-cms/registry-moderation@3077

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@3077

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@3077

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@3077

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@3077

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@3077

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@3077

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@3077

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@3077

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@3077

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@3077

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@3077

commit: 72970d3

@github-actions github-actions Bot added the review/needs-review No maintainer or bot review yet label Sep 12, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right change for #1350 and fits EmDash's architecture well. By validating registry config while Astro evaluates the integration and by returning a safe, value-free diagnostic from the runtime manifest, it turns a generic admin 500 into a field-level banner that tells the operator what to fix.

I checked the diff against the implementation, the manifest route, the admin shell and router, the registry browse/detail components, and the relevant tests. The core retry/tests look good: malformed URLs, insecure non-local aggregators, and invalid minimumReleaseAge are all surfaced as RegistryConfigurationError diagnostics, and unexpected non-configuration failures are still rethrown rather than swallowed.

The main gap is that policy.minimumReleaseAgeExclude is not validated as an array before .map() is called, so a plausible misconfiguration (e.g., passing a bare string instead of an array) still crashes with a generic TypeError instead of a field-specific REGISTRY_MINIMUM_RELEASE_AGE_EXCLUDE_INVALID diagnostic. I also have a small suggestion to make the admin banner's switch default less misleading if new registry fields are added later.

Overall this is close to clean once the array-shape handling (and a matching test) is added.


Findings

  • [needs fixing] packages/core/src/registry/config.ts:365-368

    The code assumes config.policy.minimumReleaseAgeExclude is an array and calls .map() on it without an Array.isArray() guard. If a user passes a bare string, number, or other non-array value, the normalization throws a generic TypeError: config.policy.minimumReleaseAgeExclude.map is not a function instead of the field-specific REGISTRY_MINIMUM_RELEASE_AGE_EXCLUDE_INVALID diagnostic the PR is introducing. That contradicts the goal of surfacing every malformed experimental.registry field safely.

    Validate the shape first and throw a registry configuration error, then add a unit test that asserts resolveManifestRegistryConfig returns the excluded-field code for a non-array value.

    	if (config.policy?.minimumReleaseAgeExclude !== undefined) {
    		if (!Array.isArray(config.policy.minimumReleaseAgeExclude)) {
    			throw new RegistryConfigurationError(
    				"REGISTRY_MINIMUM_RELEASE_AGE_EXCLUDE_INVALID",
    				"experimental.registry.policy.minimumReleaseAgeExclude",
    				"must be an array of DIDs or <did>/<slug> entries",
    			);
    		}
    		// Normalize at load time so callers (browser and server) can do
    		// plain string compares without each one re-implementing the
    		// case-folding rule.
    		const list = config.policy.minimumReleaseAgeExclude.map((entry) => {
    
  • [suggestion] packages/admin/src/components/RegistryConfigurationBanner.tsx:19

    The default branch assumes every unknown field is experimental.registry.aggregatorUrl. The union is currently closed to three literals so this won't mislead today, but if a new diagnostic field is added later the default will silently point the user to the wrong config key. Make the aggregatorUrl case explicit and use a generic fallback, or rely on TypeScript exhaustiveness so adding a new field forces an update here.

    		switch (error.field) {
    			case "experimental.registry.aggregatorUrl":
    				description = t`Check experimental.registry.aggregatorUrl in astro.config.mjs, then restart EmDash.`;
    				break;
    			case "experimental.registry.policy.minimumReleaseAge":
    				description = t`Check experimental.registry.policy.minimumReleaseAge in astro.config.mjs, then restart EmDash.`;
    				break;
    			case "experimental.registry.policy.minimumReleaseAgeExclude":
    				description = t`Check experimental.registry.policy.minimumReleaseAgeExclude in astro.config.mjs, then restart EmDash.`;
    				break;
    		}
    

@github-actions github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond and removed review/needs-review No maintainer or bot review yet labels Sep 12, 2026
@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Sep 12, 2026
@ascorbic ascorbic added the bot:review Trigger an emdashbot code review on this PR label Sep 12, 2026
@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Sep 12, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right fix for #1350 and fits EmDash’s architecture well. By validating experimental.registry while Astro evaluates the integration and by having the runtime manifest route return a safe, value-free diagnostic, the change turns a generic admin 500 into a field-level banner that tells the operator exactly what to correct.

The re-review confirms the two blockers from the previous pass are resolved:

  • config.policy.minimumReleaseAgeExclude is now guarded with Array.isArray() before .map(), and non-string entries also get the REGISTRY_MINIMUM_RELEASE_AGE_EXCLUDE_INVALID diagnostic.
  • RegistryConfigurationBanner now handles all three known fields explicitly and falls back to a generic experimental.registry message for future/unknown fields, avoiding pointing users at the wrong config key.

I checked the diff against registry/config.ts, the runtime manifest builder, the Astro integration, EmDashManifest/AdminManifest, the admin router/Shell, the new banner component, and the unit/browser tests. The approach is sound: recognized misconfigurations surface with a stable code + field name that leaks no configured values or credentials; unexpected failures are still rethrown rather than swallowed; no new queries are added; all admin UI strings are Lingui-wrapped; the changeset is accurate and proportional for a patch.

No further blockers or nits remain—LGTM.

@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-rereview Author pushed changes since the last review labels Sep 12, 2026
@ascorbic
ascorbic merged commit 27e432e into main Sep 12, 2026
57 checks passed
@ascorbic
ascorbic deleted the codex/1350-registry-config-errors branch September 12, 2026 10:45
@emdashbot emdashbot Bot mentioned this pull request Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant